feat(security): guard-sync + PR-body HMAC grant v2 (pairs orama #260) - #320
Conversation
…sh range Sync orama #255 CodeRabbit reviews 4835024659/4835288649 (dd8bcba3, 458a5ab9): - git -C rev-parse for linked worktree siblings - exact CLI arity (reject surplus args) - fail-closed when checker missing in sync + pre-push - scan full outgoing commit range for scripts/git/ touches - 5 pytest cases including worktree + surplus-arg
Working memory: CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md Lessons: markdownlint cli2 per-dir config, guard-sync fail-closed, PR-body Layer 0 operator grant path.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change hardens guard synchronization and repository hooks, replaces plaintext PR body overrides with HMAC-bound operator grants, adds fail-closed hook enforcement and regression tests, and records the remediation in tracked agent memory. ChangesGuard and PR body remediation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant GrantScript
participant GuardHook
participant AppendScript
participant GitHub
Operator->>GrantScript: Create interactive HMAC append grant
GrantScript-->>Operator: Write acknowledgement
AppendScript->>GuardHook: Request PR body decision
GuardHook-->>AppendScript: Allow only validated append operation
AppendScript->>GitHub: Read current PR body
AppendScript->>GitHub: Apply append-only update
AppendScript->>GuardHook: Finalize grant state
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
Deep bug scan — no critical bugs found
Scope reviewed: 72750cf..42879486 (3 commits) — guard-sync hardening (worktree discovery, fail-closed checker, full outgoing pre-push range), PR-body Layer 0 (operator TTY grant, chained-shell segment deny, fail-closed hooks), memory/docs sync.
Paths traced:
.githooks/pre-push→check-guard-sync-divergence.sh→ manifestGUARD_PARITY_REQUIREDsync-attribution-guard-scripts.shfail-closed when checker missingpr-body-guard-core.py→before-mcp-pr-body-guard.sh/before-shell-pr-body-guard.sh→append-pr-body.sh/grant-pr-body-human-override.sh
Validation:
pytest tests/test_check_guard_sync_divergence.py— 5/5 passpr-body-guard-core.pysmoke:update_pr+body → DENY;post_comment/gh pr comment→ ALLOW; chainedgh pr edit --body→ DENY- Manual divergence fixture (sibling ahead on
audit_engine.py) → exit 1 as expected - New JSONL memory rows parse cleanly
Verdict: No data-loss, crash, auth-bypass, or significant user-facing breakage found. No fix PR opened.
Non-critical (not opened): append-pr-body.sh checks grant marker but not 8h TTL (Python hooks enforce TTL); BACKUP| emission removed from guard-core (hook handlers are now dead code).
Sent by Cursor Automation: Find critical bugs
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
scripts/cursor/hooks/before-shell-pr-body-guard.sh (1)
13-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBoth hooks duplicate the guard-invocation block. The capture, fail-closed status check, and directive parser are identical in the two files. Only the guard mode and the backup reason differ. A future fix must be applied twice. Move the block into
pr-body-backup-lib.sh, or a new sourced helper, as a function that takes the mode and the reason.
scripts/cursor/hooks/before-shell-pr-body-guard.sh#L13-L42: replace the block with a call such aspr_body_run_guard shell shell-preflight.scripts/cursor/hooks/before-mcp-pr-body-guard.sh#L13-L42: replace the block withpr_body_run_guard manage_pr mcp-preflight.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/cursor/hooks/before-shell-pr-body-guard.sh` around lines 13 - 42, The guard invocation, fail-closed handling, and directive parsing are duplicated across both hooks. Add a shared function such as pr_body_run_guard in pr-body-backup-lib.sh (or a sourced helper) accepting the guard mode and backup reason, then replace scripts/cursor/hooks/before-shell-pr-body-guard.sh lines 13-42 with a call using shell and shell-preflight, and scripts/cursor/hooks/before-mcp-pr-body-guard.sh lines 13-42 with a call using manage_pr and mcp-preflight.scripts/cursor/hooks/pr-body-guard-core.py (1)
70-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove
BACKUPhandling and_normalize_github_repo_slug.No
pr-body-guard-core.pydecision emits aBACKUP|directive, so both hook branches only parse dead output while callingpr_body_backup_if_needed. Remove_normalize_github_repo_slugand the unused hook cases, or restore the guard emission.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/cursor/hooks/pr-body-guard-core.py` around lines 70 - 78, Remove the unused _normalize_github_repo_slug helper and the BACKUP-related hook branches that parse dead BACKUP| directives while invoking pr_body_backup_if_needed. Keep the remaining pr-body-guard decision handling unchanged; do not restore guard emission..agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md (1)
152-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRecord durable lesson provenance.
“See
learn.pyruns in session” does not identify the command, run result, or source record. Add the relevant command and result, or link each theme to its graduated candidate and lesson files.As per PR objectives, the report should preserve evidence, decisions, verification commands, propagation status, and documented resolutions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md around lines 152 - 158, Update the “Lessons graduated (this wave)” section to preserve provenance for each listed theme: include the relevant learn.py command and result, or link each theme to its graduated candidate and lesson files. Retain the existing themes while ensuring the report records supporting evidence and propagation status.tests/test_check_guard_sync_divergence.py (1)
131-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTighten the assertion to actually prove worktree discovery.
Line 153 checks
"pt-linked" in result.stdout or "Perpetua-Tools" in result.stdout.sibling(Perpetua-Tools) is a plain directory placed directly underworkspace, so it is discoverable by the pre-existing, non-worktree code path regardless of whether the new linked-worktree discovery in_collect_targetsworks. The test can pass even ifpt-linkedis never scanned, so it does not reliably guard against a regression in the worktree-specific fix.Assert specifically on
"pt-linked"to verify the new discovery path.✅ Proposed fix to assert on the worktree-specific target
- assert "pt-linked" in result.stdout or "Perpetua-Tools" in result.stdout + assert "pt-linked" in result.stdout assert "byte-identical" in result.stdout🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_check_guard_sync_divergence.py` around lines 131 - 154, Tighten the assertion in test_linked_worktree_sibling_discovered to require "pt-linked" in result.stdout, removing the fallback assertion for "Perpetua-Tools". Keep the existing return-code and "byte-identical" assertions unchanged so the test specifically verifies linked-worktree discovery..githooks/pre-push (1)
22-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated range computation between the guard-sync scan and the attribution audit loop.
Lines 31-39 recompute the push range for each ref, and lines 94-102 (unchanged) compute the same range again for the same refs later in the same file. Both blocks read from the same
push_refsstream and deriveguard_range/rangewith identical logic. Any future fix to one block (for example, the merge-commit issue above) risks being applied to only one of the two copies.Extract a shared helper so both loops call the same range logic.
♻️ Proposed refactor to share range computation
upstream_ref="${GIT_PUSH_UPSTREAM:-origin/main}" +range_for_ref() { + local local_oid="$1" remote_oid="$2" + if [[ "$remote_oid" == "$zero" ]]; then + if git rev-parse --verify "$upstream_ref" >/dev/null 2>&1; then + echo "$(git rev-parse "$upstream_ref")..${local_oid}" + else + echo "${local_oid}" + fi + else + echo "${remote_oid}..${local_oid}" + fi +} + # Guard-sync anti-clobber: block push when siblings carry guard mutations absent # from canonical history (see check-guard-sync-divergence.sh). guard_touch=0 while read -r local_ref local_oid remote_ref remote_oid; do [[ "$local_ref" =~ refs/heads/ ]] || continue [[ "$local_oid" == "$zero" ]] && continue - if [[ "$remote_oid" == "$zero" ]]; then - if git rev-parse --verify "$upstream_ref" >/dev/null 2>&1; then - guard_range="$(git rev-parse "$upstream_ref")..${local_oid}" - else - guard_range="${local_oid}" - fi - else - guard_range="${remote_oid}..${local_oid}" - fi + guard_range="$(range_for_ref "$local_oid" "$remote_oid")"Then reuse
range_for_ref "$local_oid" "$remote_oid"at lines 94-102 instead of recomputingrangeinline.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.githooks/pre-push around lines 22 - 65, Extract the duplicated push-range derivation into a shared range_for_ref helper using the existing upstream_ref and ref OIDs. Update both the guard-sync scan and the later attribution audit loop to call range_for_ref with the local and remote OIDs, preserving the current handling for deleted remote refs and unavailable upstream history.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agent/memory/candidates/graduated/8c5f6349aa87.json:
- Line 5: Update the accepted lesson identified as lesson_4c914783ec46 to remove
direct CURSOR_PR_BODY_HUMAN_OVERRIDE_ACK=1 authorization and mark it superseded
by lesson_8c5f6349aa87, or state that only grant-pr-body-human-override.sh
establishes the acknowledgment; then regenerate the derived ledger so memory
retrieval exposes only the operator TTY-gated authorization path.
In
@.agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md:
- Around line 22-23: Update the logical batch count in the report introduction
from four to five so it matches the defined Batch A through Batch E sections and
the existing commit map.
- Around line 31-33: Update the Markdownlint configuration guidance to state
that cli2 configuration files replace the legacy root .markdownlint.json, while
retaining the required bin/orama-system/.markdownlint-cli2.jsonc per-directory
override; remove the contradictory instruction that cli2 configuration must
exist only at the repository root.
In @.githooks/pre-push:
- Around line 45-51: Update the git diff-tree invocation inside the guard-touch
scan loop to include merge commits, using -m or the intended first-parent diff
mode. Preserve the existing scripts/git/ path filter and guard_touch control
flow.
In `@scripts/cursor/append-pr-body.sh`:
- Around line 126-131: Unify grant validation across both consumers: in
scripts/cursor/append-pr-body.sh lines 126-131, delegate validation to
pr-body-guard-core.py or enforce the same issued-at parsing and 8-hour expiry
rule; in scripts/cursor/hooks/pr-body-guard-core.py lines 49-67, require
operator-grant-v1 to appear on its own line while retaining the shared TTL
check. Ensure both paths reject expired grants and marker mentions embedded in
other text.
In `@scripts/cursor/grant-pr-body-human-override.sh`:
- Around line 6-9: Update the TTY validation condition in
grant-pr-body-human-override so execution is rejected when either stdin or
stdout is not a TTY, requiring both descriptors to be interactive; preserve the
existing error message and exit behavior.
In `@scripts/cursor/hooks/pr-body-guard-core.py`:
- Around line 125-141: Update _shell_segments to split command lines on newlines
and single pipes in addition to &&, ;, and ||. In _segment_denies_pr_body_write,
evaluate all gh api and ManagePullRequest deny patterns before the gh pr comment
and append-pr-body.sh allow rules, preserving the human override requirement for
append-pr-body.sh.
---
Nitpick comments:
In
@.agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md:
- Around line 152-158: Update the “Lessons graduated (this wave)” section to
preserve provenance for each listed theme: include the relevant learn.py command
and result, or link each theme to its graduated candidate and lesson files.
Retain the existing themes while ensuring the report records supporting evidence
and propagation status.
In @.githooks/pre-push:
- Around line 22-65: Extract the duplicated push-range derivation into a shared
range_for_ref helper using the existing upstream_ref and ref OIDs. Update both
the guard-sync scan and the later attribution audit loop to call range_for_ref
with the local and remote OIDs, preserving the current handling for deleted
remote refs and unavailable upstream history.
In `@scripts/cursor/hooks/before-shell-pr-body-guard.sh`:
- Around line 13-42: The guard invocation, fail-closed handling, and directive
parsing are duplicated across both hooks. Add a shared function such as
pr_body_run_guard in pr-body-backup-lib.sh (or a sourced helper) accepting the
guard mode and backup reason, then replace
scripts/cursor/hooks/before-shell-pr-body-guard.sh lines 13-42 with a call using
shell and shell-preflight, and scripts/cursor/hooks/before-mcp-pr-body-guard.sh
lines 13-42 with a call using manage_pr and mcp-preflight.
In `@scripts/cursor/hooks/pr-body-guard-core.py`:
- Around line 70-78: Remove the unused _normalize_github_repo_slug helper and
the BACKUP-related hook branches that parse dead BACKUP| directives while
invoking pr_body_backup_if_needed. Keep the remaining pr-body-guard decision
handling unchanged; do not restore guard emission.
In `@tests/test_check_guard_sync_divergence.py`:
- Around line 131-154: Tighten the assertion in
test_linked_worktree_sibling_discovered to require "pt-linked" in result.stdout,
removing the fallback assertion for "Perpetua-Tools". Keep the existing
return-code and "byte-identical" assertions unchanged so the test specifically
verifies linked-worktree discovery.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c789f46-7d13-48ea-8b3c-902cc29d2f21
📒 Files selected for processing (20)
.agent/memory/candidates/graduated/64e972d330c2.json.agent/memory/candidates/graduated/82c94865243e.json.agent/memory/candidates/graduated/8c5f6349aa87.json.agent/memory/episodic/AGENT_LEARNINGS.jsonl.agent/memory/semantic/LESSONS.md.agent/memory/semantic/lessons.jsonl.agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md.cursor/rules/append-only-pr-body.mdc.cursor/rules/pr-body-comment-only.mdc.githooks/pre-pushscripts/cursor/append-pr-body.shscripts/cursor/grant-pr-body-human-override.shscripts/cursor/hooks/before-mcp-pr-body-guard.shscripts/cursor/hooks/before-shell-pr-body-guard.shscripts/cursor/hooks/pr-body-backup-lib.shscripts/cursor/hooks/pr-body-guard-core.pyscripts/git/check-guard-sync-divergence.shscripts/git/remind-pr-body-append-only.shscripts/git/sync-attribution-guard-scripts.shtests/test_check_guard_sync_divergence.py
| while read -r sha; do | ||
| [[ -z "$sha" ]] && continue | ||
| if git diff-tree --no-commit-id --name-only -r "$sha" | grep -qE '^scripts/git/'; then | ||
| guard_touch=1 | ||
| break | ||
| break 2 | ||
| fi | ||
| done <"$push_refs" | ||
| if [[ "$guard_touch" == "1" ]] && [[ "${GUARD_SYNC_SKIP_DIVERGENCE_CHECK:-0}" != "1" ]]; then | ||
| WORKSPACE_ROOT="${WORKSPACE_ROOT:-$(cd "$ROOT/.." && pwd)}" \ | ||
| bash "$ROOT/scripts/git/check-guard-sync-divergence.sh" --workspace || { | ||
| echo "pre-push: blocked — guard-sync divergence (promote siblings upstream first)" >&2 | ||
| echo " See: bin/orama-system/skills/guard-sync-divergence-guard/SKILL.md" >&2 | ||
| exit 1 | ||
| } | ||
| done < <(git rev-list "$guard_range") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
d=$(mktemp -d); cd "$d"
git init -q -b main .
git config user.email t@t.com; git config user.name t
mkdir -p scripts/git
echo a > a.txt && git add a.txt && git commit -q -m init
git checkout -q -b feature
echo b > scripts/git/guard.sh && git add scripts/git/guard.sh && git commit -q -m "feature touches guard"
git checkout -q main
echo c > c.txt && git add c.txt && git commit -q -m "main change"
git merge --no-ff -q feature -m merge
merge_sha=$(git rev-parse HEAD)
echo "without -m:"; git diff-tree --no-commit-id --name-only -r "$merge_sha"
echo "with -m:"; git diff-tree --no-commit-id --name-only -r -m "$merge_sha"Repository: diazMelgarejo/Perpetua-Tools
Length of output: 214
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "File exists and relevant line context:"
if [ -f .githooks/pre-push ]; then
nl -ba .githooks/pre-push | sed -n '35,55p'
else
echo ".githooks/pre-push not found"
fi
echo
echo "Git diff-tree merge commit behavior probe:"
d=$(mktemp -d)
cd "$d"
git init -q -b main .
git config user.email t@t.com
git config user.name t
mkdir -p scripts/git
echo a > a.txt
git add a.txt
git commit -q -m init
git checkout -q -b feature
echo b > scripts/git/guard.sh
git add scripts/git/guard.sh
git commit -q -m "feature touches guard"
git checkout -q main
echo c > c.txt
git add c.txt
git commit -q -m "main change"
git merge --no-ff -q feature -m merge
merge_sha=$(git rev-parse HEAD)
{
echo "without -m:"
git diff-tree --no-commit-id --name-only -r "$merge_sha" || true
echo "empty?$( [ -z "$(git diff-tree --no-commit-id --name-only -r "$merge_sha")" ] && echo y || echo n )"
echo "with -m:"
git diff-tree --no-commit-id --name-only -r -m "$merge_sha" || true
}
rm -rf "$d"Repository: diazMelgarejo/Perpetua-Tools
Length of output: 245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "File exists and relevant line context:"
if [ -f .githooks/pre-push ]; then
sed -n '35,55p' .githooks/pre-push
else
echo ".githooks/pre-push not found"
fi
echo
echo "Git diff-tree merge commit behavior probe:"
d=$(mktemp -d)
cd "$d"
git init -q -b main .
git config user.email t@t.com
git config user.name t
mkdir -p scripts/git
echo a > a.txt
git add a.txt
git commit -q -m init
git checkout -q -b feature
echo b > scripts/git/guard.sh
git add scripts/git/guard.sh
git commit -q -m "feature touches guard"
git checkout -q main
echo c > c.txt
git add c.txt
git commit -q -m "main change"
git merge --no-ff -q feature -m merge
merge_sha=$(git rev-parse HEAD)
{
echo "without -m:"
git diff-tree --no-commit-id --name-only -r "$merge_sha" || true
without="$(git diff-tree --no-commit-id --name-only -r "$merge_sha" || true)"
if [ -z "$without" ]; then
echo "empty?y"
else
echo "empty?n"
fi
echo "with -m:"
git diff-tree --no-commit-id --name-only -r -m "$merge_sha" || true
}
rm -rf "$d"Repository: diazMelgarejo/Perpetua-Tools
Length of output: 895
Include merge commits in the guard-touch scan.
git diff-tree --name-only -r "$sha" omits merge commits by default. If an outgoing merge resolves changes in a scripts/git/ file, this loop leaves guard_touch as 0 and skips the divergence check. Add -m to the diff-tree call, or use --diff-merges=first-parent if you only mean to scan the first-parent path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.githooks/pre-push around lines 45 - 51, Update the git diff-tree invocation
inside the guard-touch scan loop to include merge commits, using -m or the
intended first-parent diff mode. Preserve the existing scripts/git/ path filter
and guard_touch control flow.
Synced via scripts/git/sync-attribution-guard-scripts.sh from orama branch 2026-08-02-pr-body-grant-hmac-mvp: grant lib, hooks, append script, pre-push range_for_ref, guard-sync test, PR-body tests. CODERABBIT wave doc provenance updated.
Saga weld checklist; README indexes local-only EXA exports (not committed due to hygiene). Pairs with orama 2026-08-02-pr-body-grant-hmac-mvp.
Welded with orama #260 (2026-08-02)Paired PR: diazMelgarejo/orama-system#260 Added on this branch (3 commits since
|
Replay state machine, GH_BIN append test, saga path hygiene for CI gate.
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (6)
scripts/cursor/hooks/pr-body-guard-core.py (1)
166-168: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueApply the Ruff RUF005 suggestion.
Use unpacking instead of list concatenation.
♻️ Proposed refactor
if backup_lines: - return backup_lines + ["ALLOW"] + return [*backup_lines, "ALLOW"] return ["ALLOW"]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/cursor/hooks/pr-body-guard-core.py` around lines 166 - 168, Update the return logic in the backup-lines handling block to use iterable unpacking instead of list concatenation, while preserving the existing “ALLOW” suffix and fallback return when backup_lines is empty.Source: Linters/SAST tools
tests/test_append_pr_body_grant_flow.py (2)
36-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winParse
--body-fileinstead of relying on the positional$7.The fake
ghreads the merged body from$7. That works only whileappend-pr-body.shcallsgh pr edit "$pr_number" --repo "$repo_slug" --body-file "$out"in exactly that order. If the flag order changes,cat "$7"fails and the test reports a generic non-zero exit instead of the real cause. Scan the arguments for--body-file.♻️ Proposed refactor
if [[ "$1" == pr && "$2" == edit ]]; then - cat "$7" > '{body_file}' + shift 2 + body_path="" + while [[ $# -gt 0 ]]; do + if [[ "$1" == --body-file ]]; then + body_path="$2" + break + fi + shift + done + [[ -n "$body_path" ]] || {{ echo "fake gh: no --body-file" >&2; exit 1; }} + cat "$body_path" > '{body_file}' exit 0 fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_append_pr_body_grant_flow.py` around lines 36 - 53, Update the fake gh script in the test setup to scan its arguments for the --body-file option and use the following argument as the body path, instead of assuming the path is positional $7. Preserve the existing pr edit behavior and unexpected-command failure handling.
96-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the resulting PR body content.
The test checks the exit code, the
updated:line, and that the grant no longer verifies. It does not check that the follow-up block reached the PR body. Read the fakeghbody file and assert that it contains## Follow-up: testandoperator note. That assertion also protects the reconcile format contract inpr-body-grant-lib.py.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_append_pr_body_grant_flow.py` around lines 96 - 113, Extend the test after the append command succeeds by reading the fake gh PR body file and asserting it contains both “## Follow-up: test” and “operator note”. Keep the existing return-code, updated-output, and grant-consumption assertions unchanged; anchor the change in the test flow around the `consume_check` setup and the fake body artifact.scripts/cursor/grant-pr-body-human-override.sh (1)
19-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReport a usage error when the positional arguments are missing.
shift 2 || truehides the failure when the caller passes fewer than two arguments. With one argument,repo_slugholds that argument and$1still holds it, so the loop reportsunknown argumentinstead of the usage message. Check the argument count first.♻️ Proposed refactor
+if [[ $# -lt 2 && "${1:-}" != "-h" && "${1:-}" != "--help" ]]; then + echo "error: usage: grant-pr-body-human-override.sh <owner/repo> <pr-number> --file|--message" >&2 + exit 1 +fi repo_slug="${1:-}" pr_number="${2:-}" shift 2 || true🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/cursor/grant-pr-body-human-override.sh` around lines 19 - 21, Validate that at least two positional arguments are present before assigning or shifting arguments in the script’s argument-parsing flow. When fewer than two arguments are supplied, emit the existing usage error and exit; otherwise preserve the current repo_slug, pr_number, and remaining-argument handling.tests/test_pr_body_grant_lib.py (2)
130-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTighten the replay assertion.
"nonce" in err2.lower() or "grant" in err2.lower()matches nearly every error string the library returns, including a missing ack file or an HMAC failure. The test therefore passes even when the replay path is not the reason. Assert the exact replay message.♻️ Proposed refactor
assert not ok2 - assert "nonce" in err2.lower() or "grant" in err2.lower() + assert "already consumed" in err2🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_pr_body_grant_lib.py` around lines 130 - 138, In the replay assertion for verify_grant_for_append, replace the broad nonce-or-grant substring check with an exact assertion against the expected replay error message. Keep the existing failure assertion and ensure the test specifically validates the replay path rather than unrelated verification errors.
49-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a TTL expiry test.
The 8-hour TTL is a core control of grant v2, and
_grant_ttl_okhas no test. A regression that widens or removes the window would go undetected. Mint a grant, rewriteissued-atto a timestamp older thanGRANT_TTL_SECONDS, and assert that verification reports the expiry error. Add a case for a futureissued-atas well, because_grant_ttl_okrejects a negative age.Do you want me to generate these test cases?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_pr_body_grant_lib.py` around lines 49 - 60, Add TTL coverage alongside test_mint_and_verify_happy_path: mint a grant, rewrite its issued-at timestamp to older than GRANT_TTL_SECONDS, and assert verification fails with the expiry error; also add a future issued-at case and assert the negative-age rejection. Reuse the existing grant fixture and grant serialization/parsing helpers rather than changing production code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
@.agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md:
- Around line 124-132: Update the Batch F report metadata to consistently
reflect August 2, 2026, including the document title, current branch tip, and
commit map beyond the earlier Batch E tip 88783688; alternatively, clearly label
Batch F as a dated addendum while preserving accurate branch and commit
references.
In @.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md:
- Around line 4-8: Wrap the overlong Markdown lines in the document, including
the metadata lines containing the branch, trigger chain, canonical plan, and
research entries and the additional violations at lines 163 and 178-179, so
every line is at most 100 columns while preserving the existing wording and
Markdown structure.
- Around line 110-115: Update the verification commands in
.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md lines 110-115
and
.agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md
lines 170-174 to include tests/test_append_pr_body_grant_flow.py, ensuring the
append-flow regression test runs before recording verification results or pass
counts.
- Around line 3-5: Rename the paired branches to the required
yyyy-mm-dd-NNN-brief-summary format and update every tracked reference. In
.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md lines 3-5, 32,
78-79, and 157-158, update both branch declarations and repeated references;
update the Batch F branch reference in
.agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md
lines 126-127 and the canonical/mirror entries in
.agent/memory/working/WORKSPACE.md lines 10-11.
- Around line 175-179: Update the active doctrine in
scripts/cursor/hooks/before-submit-pr-body-reminder.sh to describe the v2-only,
hookified grant and remove the CURSOR_PR_BODY_HUMAN_OVERRIDE_ACK exception. Then
review related .agent/memory references, marking obsolete v1/env-override
guidance as historical or updating it to the v2 behavior.
In @.agent/memory/working/WORKSPACE.md:
- Line 38: Update the PT `#320` follow-up in .agent/memory/working/WORKSPACE.md at
line 38 and .agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md at
line 177 to state that PR body writes require an operator-minted v2 grant:
without one, use ManagePullRequest post_comment only; with one, use
append-pr-body.sh and the exact granted content.
In `@scripts/cursor/append-pr-body.sh`:
- Around line 244-262: Update the state tracking around the `gh pr edit`,
`mark_cmd`, and `consume_cmd` flow so a successful PR body update records that
the remote mutation has occurred before finalization begins. Ensure the
`release_on_fail` trap does not release the reservation after `gh pr edit`
succeeds, even if either finalization command fails, while preserving release
behavior when the edit itself fails.
In `@scripts/cursor/hooks/pr-body-backup-lib.sh`:
- Around line 63-95: Update the guard runner around pr-body-guard-core.py to
capture stderr separately from the decision output while preserving the nonzero
exit fail-closed behavior. In the guard_output parsing case statement, add a
default branch that sets PR_BODY_GUARD_DECISION to DENY with an internal-error
message and stops parsing, and immediately break after any DENY or failed
pr_body_backup_if_needed result so later lines cannot trigger additional work.
- Around line 51-53: Update pr_body_backup_dir() and the backup preflight to
reject unresolved or unsafe fallback directories instead of defaulting to
${TMPDIR:-/tmp}; only accept an existing TMPDIR that maps to the known-safe
backup tree, and ensure PR body backup is denied before mkdir -p or any write
when that validation fails.
In `@scripts/cursor/hooks/pr-body-guard-core.py`:
- Around line 97-135: Update _shell_segments to split on single pipes in
addition to the existing separators, and reorder _segment_inspect so the gh pr
edit, gh api, and ManagePullRequest deny checks run before the gh pr comment and
append-pr-body.sh allow checks. Add regression tests in
tests/test_pr_body_guard_core.py covering pipe-separated commands, matching the
existing newline cases.
In `@scripts/cursor/pr-body-grant-lib.py`:
- Around line 185-203: Gate the PR_BODY_GRANT_HMAC_SECRET override in
resolve_hmac_secret behind an explicit shared test-mode marker, while preserving
normal environment, Keychain, and fallback-file resolution otherwise. In
scripts/cursor/append-pr-body.sh at lines 121-125, honor GH_BIN only when that
same marker is set and use gh by default; update
tests/test_append_pr_body_grant_flow.py and tests/test_pr_body_grant_lib.py to
set the marker.
- Around line 724-754: Update parse_append_segment to use shlex.split when
tokenizing rest instead of rest.split, preserving shell-style quoted values for
--message and --title; retain the existing option scan and return behavior using
the parsed tokens.
- Around line 702-716: Update mint_grant before digest/signature generation and
ACK file construction to validate repo and pr_number, rejecting values
containing newline or pipe delimiters. Preserve the existing output for valid
values and fail before signing or writing the ACK when either field is invalid.
In `@scripts/git/sync-attribution-guard-scripts.sh`:
- Around line 220-229: Update the synchronization loops for the required PR-body
guard files and cursor_hook_rel to fail closed when a source file is absent:
replace the silent continue with an error report and nonzero exit. If any files
are intentionally optional, separate them into an explicit optional list and
report each skipped entry.
- Around line 231-240: Update atomic_install_file to validate every existing
destination ancestor, including dirname "$dest", and reject symlinked parent
directories before creating its temporary file or moving the installed file.
Preserve normal nested hooks/... installation through the sync loop, and add a
regression test covering a symlinked $target/scripts/cursor/hooks directory to
ensure no file is written outside the target.
In `@tests/test_pr_body_guard_core.py`:
- Around line 42-45: Update test_append_pr_body_denied_without_grant to request
the grant_setup fixture, ensuring the guard uses isolated test state instead of
the real home-directory grant and nonce files. Keep the existing command and
denial assertion unchanged.
---
Nitpick comments:
In `@scripts/cursor/grant-pr-body-human-override.sh`:
- Around line 19-21: Validate that at least two positional arguments are present
before assigning or shifting arguments in the script’s argument-parsing flow.
When fewer than two arguments are supplied, emit the existing usage error and
exit; otherwise preserve the current repo_slug, pr_number, and
remaining-argument handling.
In `@scripts/cursor/hooks/pr-body-guard-core.py`:
- Around line 166-168: Update the return logic in the backup-lines handling
block to use iterable unpacking instead of list concatenation, while preserving
the existing “ALLOW” suffix and fallback return when backup_lines is empty.
In `@tests/test_append_pr_body_grant_flow.py`:
- Around line 36-53: Update the fake gh script in the test setup to scan its
arguments for the --body-file option and use the following argument as the body
path, instead of assuming the path is positional $7. Preserve the existing pr
edit behavior and unexpected-command failure handling.
- Around line 96-113: Extend the test after the append command succeeds by
reading the fake gh PR body file and asserting it contains both “## Follow-up:
test” and “operator note”. Keep the existing return-code, updated-output, and
grant-consumption assertions unchanged; anchor the change in the test flow
around the `consume_check` setup and the fake body artifact.
In `@tests/test_pr_body_grant_lib.py`:
- Around line 130-138: In the replay assertion for verify_grant_for_append,
replace the broad nonce-or-grant substring check with an exact assertion against
the expected replay error message. Keep the existing failure assertion and
ensure the test specifically validates the replay path rather than unrelated
verification errors.
- Around line 49-60: Add TTL coverage alongside test_mint_and_verify_happy_path:
mint a grant, rewrite its issued-at timestamp to older than GRANT_TTL_SECONDS,
and assert verification fails with the expiry error; also add a future issued-at
case and assert the negative-age rejection. Reuse the existing grant fixture and
grant serialization/parsing helpers rather than changing production code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b9ddb288-4acf-4045-bdb3-6b09829752ac
📒 Files selected for processing (19)
.agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md.agent/memory/working/PR_BODY_GRANT_HMAC_DECISIONS_2026-08-02.jsonl.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md.agent/memory/working/WORKSPACE.md.agent/references/pr-body-grant-research-raw-2026-08-02/README.md.cursor/rules/never-undo-attribution-expunge.mdc.githooks/pre-pushscripts/cursor/append-pr-body.shscripts/cursor/grant-pr-body-human-override.shscripts/cursor/hooks/before-mcp-pr-body-guard.shscripts/cursor/hooks/before-shell-pr-body-guard.shscripts/cursor/hooks/pr-body-backup-lib.shscripts/cursor/hooks/pr-body-guard-core.pyscripts/cursor/pr-body-grant-lib.pyscripts/git/sync-attribution-guard-scripts.shtests/test_append_pr_body_grant_flow.pytests/test_check_guard_sync_divergence.pytests/test_pr_body_grant_lib.pytests/test_pr_body_guard_core.py
🚧 Files skipped from review as they are similar to previous changes (2)
- .githooks/pre-push
- tests/test_check_guard_sync_divergence.py
| > **Status:** implemented on paired branches (orama canonical, PT mirror) | ||
| > **orama-system:** branch `2026-08-02-pr-body-grant-hmac-mvp` (post-#255 `main`, commit after `525961d6`) | ||
| > **Perpetua-Tools:** branch `2026-08-02-pr-body-grant-hmac-mvp` (tracks PR #320 wave) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the required branch-name format consistently.
The paired branch name omits the required NNN sequence. Rename the branch pair and update every tracked reference.
.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md#L3-L5: rename both branch declarations and update the repeated references at Lines 32, 78-79, and 157-158..agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md#L126-L127: update the Batch F branch reference..agent/memory/working/WORKSPACE.md#L10-L11: update the canonical and mirror branch entries.
As per coding guidelines, use dated branch names in the format yyyy-mm-dd-NNN-brief-summary.
🧰 Tools
🪛 GitHub Actions: Markdown Lint / 0_Markdownlint-cli2.txt
[error] 4-4: markdownlint MD013/line-length violation: line length is 108 characters, exceeding the expected maximum of 100.
🪛 GitHub Actions: Markdown Lint / Markdownlint-cli2
[error] 4-4: markdownlint MD013/line-length violation: line length is 108 characters, exceeding the expected maximum of 100.
🪛 GitHub Check: Markdownlint-cli2
[failure] 4-4: Line length
.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md:4:101 MD013/line-length Line length [Expected: 100; Actual: 108] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md013.md
📍 Affects 3 files
.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md#L3-L5(this comment).agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md#L126-L127.agent/memory/working/WORKSPACE.md#L10-L11
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md around lines
3 - 5, Rename the paired branches to the required yyyy-mm-dd-NNN-brief-summary
format and update every tracked reference. In
.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md lines 3-5, 32,
78-79, and 157-158, update both branch declarations and repeated references;
update the Batch F branch reference in
.agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md
lines 126-127 and the canonical/mirror entries in
.agent/memory/working/WORKSPACE.md lines 10-11.
Source: Coding guidelines
| ## Open follow-ups (not blocking MVP) | ||
|
|
||
| - [ ] Open orama PR from `2026-08-02-pr-body-grant-hmac-mvp`; update PT #320 body with orama tip SHA | ||
| - [ ] Doctrine pass: hookify + `.cursor/rules` still mention v1 / env override in some places — grep `operator-grant-v1` | ||
| - [ ] `check_tdd_commit.sh` unbound `staged[@]` on empty index under `set -u` (macOS bash) — unrelated but blocked one commit attempt |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 2 \
'operator-grant-v1|CURSOR_PR_BODY_HUMAN_OVERRIDE_ACK' \
.cursor scriptsRepository: diazMelgarejo/Perpetua-Tools
Length of output: 1770
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- candidate tracked files containing references ---\n'
git ls-files | xargs rg -n 'operator-grant-v1|CURSOR_PR_BODY_HUMAN_OVERRIDE_ACK' || true
printf '\n--- relevant hook-body reminder ---\n'
sed -n '1,80p' .agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md 2>/dev/null || true
sed -n '140,200p' .agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md 2>/dev/null || true
printf '\n--- hook implementation excerpt ---\n'
sed -n '1,80p' scripts/cursor/hooks/before-submit-pr-body-reminder.shRepository: diazMelgarejo/Perpetua-Tools
Length of output: 257
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- candidate tracked files containing references ---'
git ls-files | xargs rg -n 'operator-grant-v1|CURSOR_PR_BODY_HUMAN_OVERRIDE_ACK' || true
printf '%s\n' ''
printf '%s\n' '--- relevant .agent memory excerpts ---'
if [ -f .agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md ]; then
sed -n '1,80p' .agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md
printf '%s\n' ''
sed -n '140,200p' .agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md
fi
printf '%s\n' ''
printf '%s\n' '--- hook implementation excerpt ---'
sed -n '1,80p' scripts/cursor/hooks/before-submit-pr-body-reminder.shRepository: diazMelgarejo/Perpetua-Tools
Length of output: 13452
Update active v1/env-override doctrine to v2.
scripts/cursor/hooks/before-submit-pr-body-reminder.sh still tells agents body writes are allowed when CURSOR_PR_BODY_HUMAN_OVERRIDE_ACK=1, but the grant is v2-only and hookified. Update that instruction, then mark legacy .agent/memory references as historical or update them.
🧰 Tools
🪛 GitHub Check: Markdownlint-cli2
[failure] 179-179: Line length
.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md:179:101 MD013/line-length Line length [Expected: 100; Actual: 133] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md013.md
[failure] 178-178: Line length
.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md:178:101 MD013/line-length Line length [Expected: 100; Actual: 120] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md013.md
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md around lines
175 - 179, Update the active doctrine in
scripts/cursor/hooks/before-submit-pr-body-reminder.sh to describe the v2-only,
hookified grant and remove the CURSOR_PR_BODY_HUMAN_OVERRIDE_ACK exception. Then
review related .agent/memory references, marking obsolete v1/env-override
guidance as historical or updating it to the v2 behavior.
| def parse_append_segment(segment: str) -> tuple[str, str, str | None, str | None] | None: | ||
| seg = segment.strip() | ||
| if "append-pr-body.sh" not in seg: | ||
| return None | ||
| match = APPEND_SEGMENT_RE.search(seg) | ||
| if not match: | ||
| return None | ||
| repo = match.group(1) | ||
| pr = match.group(2) | ||
| rest = (match.group(3) or "").strip() | ||
| file_path: str | None = None | ||
| message: str | None = None | ||
| tokens = rest.split() | ||
| idx = 0 | ||
| while idx < len(tokens): | ||
| token = tokens[idx] | ||
| if token == "--file" and idx + 1 < len(tokens): | ||
| file_path = tokens[idx + 1] | ||
| idx += 2 | ||
| continue | ||
| if token == "--message" and idx + 1 < len(tokens): | ||
| message = tokens[idx + 1] | ||
| idx += 2 | ||
| continue | ||
| if token in ("--title", "-h", "--help"): | ||
| idx += 2 if token == "--title" and idx + 1 < len(tokens) else 1 | ||
| continue | ||
| idx += 1 | ||
| if not file_path and not message: | ||
| return None | ||
| return repo, pr, file_path, message |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use shlex.split so quoted --message and --title values parse correctly.
rest.split() splits on whitespace and keeps quote characters. For the documented usage append-pr-body.sh <repo> <pr> --message "follow-up text", message becomes "follow-up. The guard then computes the content digest over "follow-up while the operator minted the grant over follow-up text. verify_grant_fields reports a digest mismatch and the hook denies a correctly granted append. Quoted --title values split the same way and can leave stray tokens in the scan loop.
Parse the segment with shlex so token values match what the shell passes to the script.
🐛 Proposed fix
+import shlex
+
def parse_append_segment(segment: str) -> tuple[str, str, str | None, str | None] | None:
seg = segment.strip()
if "append-pr-body.sh" not in seg:
return None
- match = APPEND_SEGMENT_RE.search(seg)
- if not match:
- return None
- repo = match.group(1)
- pr = match.group(2)
- rest = (match.group(3) or "").strip()
+ try:
+ argv = shlex.split(seg)
+ except ValueError:
+ return None
+ for pos, arg in enumerate(argv):
+ if arg.endswith("append-pr-body.sh"):
+ break
+ else:
+ return None
+ tail = argv[pos + 1 :]
+ if len(tail) < 2 or not tail[1].isdigit():
+ return None
+ repo, pr = tail[0], tail[1]
+ tokens = tail[2:]
file_path: str | None = None
message: str | None = None
- tokens = rest.split()
idx = 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def parse_append_segment(segment: str) -> tuple[str, str, str | None, str | None] | None: | |
| seg = segment.strip() | |
| if "append-pr-body.sh" not in seg: | |
| return None | |
| match = APPEND_SEGMENT_RE.search(seg) | |
| if not match: | |
| return None | |
| repo = match.group(1) | |
| pr = match.group(2) | |
| rest = (match.group(3) or "").strip() | |
| file_path: str | None = None | |
| message: str | None = None | |
| tokens = rest.split() | |
| idx = 0 | |
| while idx < len(tokens): | |
| token = tokens[idx] | |
| if token == "--file" and idx + 1 < len(tokens): | |
| file_path = tokens[idx + 1] | |
| idx += 2 | |
| continue | |
| if token == "--message" and idx + 1 < len(tokens): | |
| message = tokens[idx + 1] | |
| idx += 2 | |
| continue | |
| if token in ("--title", "-h", "--help"): | |
| idx += 2 if token == "--title" and idx + 1 < len(tokens) else 1 | |
| continue | |
| idx += 1 | |
| if not file_path and not message: | |
| return None | |
| return repo, pr, file_path, message | |
| import shlex | |
| def parse_append_segment(segment: str) -> tuple[str, str, str | None, str | None] | None: | |
| seg = segment.strip() | |
| if "append-pr-body.sh" not in seg: | |
| return None | |
| try: | |
| argv = shlex.split(seg) | |
| except ValueError: | |
| return None | |
| for pos, arg in enumerate(argv): | |
| if arg.endswith("append-pr-body.sh"): | |
| break | |
| else: | |
| return None | |
| tail = argv[pos + 1 :] | |
| if len(tail) < 2 or not tail[1].isdigit(): | |
| return None | |
| repo, pr = tail[0], tail[1] | |
| tokens = tail[2:] | |
| file_path: str | None = None | |
| message: str | None = None | |
| idx = 0 | |
| while idx < len(tokens): | |
| token = tokens[idx] | |
| if token == "--file" and idx + 1 < len(tokens): | |
| file_path = tokens[idx + 1] | |
| idx += 2 | |
| continue | |
| if token == "--message" and idx + 1 < len(tokens): | |
| message = tokens[idx + 1] | |
| idx += 2 | |
| continue | |
| if token in ("--title", "-h", "--help"): | |
| idx += 2 if token == "--title" and idx + 1 < len(tokens) else 1 | |
| continue | |
| idx += 1 | |
| if not file_path and not message: | |
| return None | |
| return repo, pr, file_path, message |
🧰 Tools
🪛 Ruff (0.16.0)
[error] 740-740: Possible hardcoded password assigned to: "token"
(S105)
[error] 744-744: Possible hardcoded password assigned to: "token"
(S105)
[error] 749-749: Possible hardcoded password assigned to: "token"
(S105)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/cursor/pr-body-grant-lib.py` around lines 724 - 754, Update
parse_append_segment to use shlex.split when tokenizing rest instead of
rest.split, preserving shell-style quoted values for --message and --title;
retain the existing option scan and return behavior using the parsed tokens.
| for cursor_rel in \ | ||
| append-pr-body.sh \ | ||
| grant-pr-body-human-override.sh \ | ||
| pr-body-grant-lib.py; do | ||
| [[ -f "$source_root/scripts/cursor/$cursor_rel" ]] || continue | ||
| atomic_install_file \ | ||
| "$source_root/scripts/cursor/$cursor_rel" \ | ||
| "$target/scripts/cursor/$cursor_rel" \ | ||
| 0755 | ||
| done |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail closed when a required guard file is missing.
Lines [224] and [236] silently skip missing source files. The synchronization can then complete with stale or missing PR-body guard code. Treat these entries as required and exit on absence. If mixed-version synchronization is intentional, maintain an explicit optional list and report every skipped file.
Proposed fail-closed check
- [[ -f "$source_root/scripts/cursor/$cursor_rel" ]] || continue
+ if [[ ! -f "$source_root/scripts/cursor/$cursor_rel" ]]; then
+ printf 'error: missing source file: %s\n' \
+ "$source_root/scripts/cursor/$cursor_rel" >&2
+ exit 1
+ fiApply the same check to cursor_hook_rel.
Also applies to: 231-241
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/git/sync-attribution-guard-scripts.sh` around lines 220 - 229, Update
the synchronization loops for the required PR-body guard files and
cursor_hook_rel to fail closed when a source file is absent: replace the silent
continue with an error report and nonzero exit. If any files are intentionally
optional, separate them into an explicit optional list and report each skipped
entry.
| for cursor_hook_rel in \ | ||
| hooks/pr-body-guard-core.py \ | ||
| hooks/pr-body-backup-lib.sh \ | ||
| hooks/before-shell-pr-body-guard.sh \ | ||
| hooks/before-mcp-pr-body-guard.sh; do | ||
| [[ -f "$source_root/scripts/cursor/$cursor_hook_rel" ]] || continue | ||
| atomic_install_file \ | ||
| "$source_root/scripts/cursor/$cursor_hook_rel" \ | ||
| "$target/scripts/cursor/$cursor_hook_rel" \ | ||
| 0755 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject symlinked parent directories before installing hook files.
Lines [231]-[240] add nested hooks/... destinations. atomic_install_file checks only the final destination. It does not reject a symlink in dirname "$dest". If $target/scripts/cursor/hooks is a symlink, mktemp and mv can write guard files outside $target. Validate all existing ancestors before creating the temporary file, and add a regression test for a symlinked hooks directory.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/git/sync-attribution-guard-scripts.sh` around lines 231 - 240, Update
atomic_install_file to validate every existing destination ancestor, including
dirname "$dest", and reject symlinked parent directories before creating its
temporary file or moving the installed file. Preserve normal nested hooks/...
installation through the sync loop, and add a regression test covering a
symlinked $target/scripts/cursor/hooks directory to ensure no file is written
outside the target.
| def test_append_pr_body_denied_without_grant(guard_core): | ||
| cmd = "bash scripts/cursor/append-pr-body.sh a/b 1 --file x.md" | ||
| lines = guard_core._shell_decision_lines(cmd) | ||
| assert lines[0].startswith("DENY") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Isolate this test from the real home directory.
test_append_pr_body_denied_without_grant does not request the grant_setup fixture. The guard therefore reads the real ~/.cursor/pr-body-human-override-ack and nonce_is_consumed creates or updates the real ~/.cursor/pr-body-grant-nonces.json. The test mutates operator state, and it would fail on a machine that holds a live grant for a/b PR 1. Add the fixture.
💚 Proposed fix
-def test_append_pr_body_denied_without_grant(guard_core):
+def test_append_pr_body_denied_without_grant(guard_core, grant_setup):
cmd = "bash scripts/cursor/append-pr-body.sh a/b 1 --file x.md"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_append_pr_body_denied_without_grant(guard_core): | |
| cmd = "bash scripts/cursor/append-pr-body.sh a/b 1 --file x.md" | |
| lines = guard_core._shell_decision_lines(cmd) | |
| assert lines[0].startswith("DENY") | |
| def test_append_pr_body_denied_without_grant(guard_core, grant_setup): | |
| cmd = "bash scripts/cursor/append-pr-body.sh a/b 1 --file x.md" | |
| lines = guard_core._shell_decision_lines(cmd) | |
| assert lines[0].startswith("DENY") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_pr_body_guard_core.py` around lines 42 - 45, Update
test_append_pr_body_denied_without_grant to request the grant_setup fixture,
ensuring the guard uses isolated test state instead of the real home-directory
grant and nonce files. Keep the existing command and denial assertion unchanged.
Update workspace, saga, decisions JSONL, CodeRabbit Batch G, semantic DECISIONS, and graduate five lessons (replay state machine, canonical payload, same-user boundary, HMAC override, path hygiene).
Mirror guard-sync manifest, scrub_dsstore.sh, grant v2 hardening, expanded tests, githooks -x guard, and WORKSPACE markdownlint fixes.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.agent/memory/semantic/lessons.jsonl (1)
981-981: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReword the rationale to avoid embedding the literal scratch-path pattern it warns against.
Line 981 (
lesson_1a6dc6bccad0) states that tracked.agentmemory "must not embed ephemeral scratch path literals." Its ownrationalefield contains the literal string/tmp("PT#320Actions failed on saga /tmp worktree names"). This repeats a documented anti-pattern in this same memory system: a rule that quotes the exact literal it forbids is self-defeating (seelesson_6c176e735ab4,lesson_9ff1243fd27c,lesson_e9d0a3deb701in this file). Reword the rationale to describe the incident without the literal path fragment, for example: "PT#320CI failed when saga notes embedded a scratch-path literal instead of a neutral worktree label."🩹 Proposed rationale rewording
-"rationale": "PT `#320` Actions failed on saga /tmp worktree names; use neutral worktree labels in memory.", +"rationale": "PT `#320` Actions failed when saga notes embedded a scratch-path literal instead of a neutral worktree label; use neutral worktree labels in memory.",Based on learnings, this repo's own memory already documents this exact class of bug: "a rule that quotes the secret it exists to forbid is self-defeating" (
lesson_6c176e735ab4), and as per coding guidelines, tracked files must not contain workstation-specific or scratch-path literals.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agent/memory/semantic/lessons.jsonl at line 981, Reword the rationale for lesson_1a6dc6bccad0 in the tracked lessons.jsonl entry to describe the scratch-worktree incident without embedding the forbidden scratch-path literal; retain the existing meaning and use a neutral worktree label instead.Sources: Coding guidelines, Learnings
🧹 Nitpick comments (2)
.agent/memory/working/PR_BODY_GRANT_HMAC_DECISIONS_2026-08-02.jsonl (1)
11-11: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse
content_digestconsistently in the HMAC contract.Line [11] uses
digest, while.agent/memory/candidates/graduated/720dedfdd2c8.jsonLine [5] andscripts/cursor/pr-body-grant-lib.pyuse thecontent_digest/grant_digestterminology. Keep one identifier across the decision, implementation, and golden-vector tests to prevent future signer-verifier drift.Proposed wording
-...|action|digest +...|action|content_digest🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agent/memory/working/PR_BODY_GRANT_HMAC_DECISIONS_2026-08-02.jsonl at line 11, Update decision D11’s canonical HMAC payload to use content_digest consistently instead of digest, matching the established content_digest/grant_digest terminology used by the implementation and golden-vector references. Preserve the fixed field order and UTF-8 signing contract..agent/memory/working/WORKSPACE.md (1)
47-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSeparate repository scope and test counts.
- Label
WORKSPACE.md’s 26 tests as an aggregate and list theoramaand PT counts.- Identify the repository for each 21-test result in
PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.mdandCODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md.- Ensure the Batch G entry states whether 21 tests passed in each repository or in one combined run.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agent/memory/working/WORKSPACE.md around lines 47 - 49, Separate the test results by repository: in .agent/memory/working/WORKSPACE.md lines 47-49, label 26 as an aggregate and list the Orama and PT counts; in .agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md lines 125-127 and 180-190, identify the repository for each 21-test result; and in .agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md lines 171-174, clarify whether Batch G’s 21 tests passed separately in each repository or in one combined run.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agent/memory/candidates/graduated/1a6dc6bccad0.json:
- Line 43: Remove the literal “/tmp” from the tracked memory text in
decisions[1].notes and rationale, replacing it with neutral wording such as
“ephemeral worktree names” while preserving the existing meaning.
In @.agent/memory/candidates/graduated/d1f3789d4592.json:
- Line 5: Update the claim around the “Human-authorized” wording to use
“operator-gated” instead, and state that the operator-grant-v2 HMAC authorizes
the append capability without proving human identity. Preserve the existing
distinctions from CURSOR_PR_BODY_HUMAN_OVERRIDE_ACK exports and
operator-grant-v1 plaintext acknowledgments.
In @.agent/memory/semantic/DECISIONS.md:
- Around line 8-12: Fix Markdownlint violations in the changed decision block:
wrap lines 8, 10, 12, and 20 to no more than 100 characters, and insert a blank
line immediately after the **Links:** heading. Preserve the existing decision
content and formatting structure.
- Line 8: Update .agent/memory/semantic/DECISIONS.md at line 8 to replace the
public-flow gh pr edit step with scripts/cursor/append-pr-body.sh, identifying
it as the guarded wrapper that performs the remote edit internally. Update
.agent/memory/semantic/LESSONS.md at line 23 to state that agents must not
invoke gh pr edit directly and may use only append-pr-body.sh after the operator
grant.
In @.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md:
- Around line 61-64: Update
.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md lines 61-64 to
state that append-pr-body.sh owns reserve, its internal gh pr edit,
mark-applied, and consume; direct gh pr edit remains denied. Update
.agent/memory/working/WORKSPACE.md lines 41-42 to replace the direct gh edit
reference with append-pr-body.sh or explicitly label it as an internal step.
In @.githooks/pre-commit:
- Around line 27-32: Update the pre-commit hygiene gate around repo_hygiene.py
so that when the script exists but python3 is unavailable, it emits an error and
exits nonzero instead of falling through to check_identity.sh. Preserve the
existing fallback only for clones where the hygiene script itself is absent,
matching the LAN gate’s failure behavior.
In `@scripts/cursor/pr-body-grant-lib.py`:
- Around line 178-180: Extend validation to reject newline characters as well as
pipe characters for both repo and pr_number values. Update _validate_repo_slug
and add or reuse a corresponding validator for pr_number, then invoke these
checks from mint_grant and verify_grant_fields before values reach
_canonical_payload or are written into the acknowledgment fields.
---
Outside diff comments:
In @.agent/memory/semantic/lessons.jsonl:
- Line 981: Reword the rationale for lesson_1a6dc6bccad0 in the tracked
lessons.jsonl entry to describe the scratch-worktree incident without embedding
the forbidden scratch-path literal; retain the existing meaning and use a
neutral worktree label instead.
---
Nitpick comments:
In @.agent/memory/working/PR_BODY_GRANT_HMAC_DECISIONS_2026-08-02.jsonl:
- Line 11: Update decision D11’s canonical HMAC payload to use content_digest
consistently instead of digest, matching the established
content_digest/grant_digest terminology used by the implementation and
golden-vector references. Preserve the fixed field order and UTF-8 signing
contract.
In @.agent/memory/working/WORKSPACE.md:
- Around line 47-49: Separate the test results by repository: in
.agent/memory/working/WORKSPACE.md lines 47-49, label 26 as an aggregate and
list the Orama and PT counts; in
.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md lines 125-127
and 180-190, identify the repository for each 21-test result; and in
.agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md
lines 171-174, clarify whether Batch G’s 21 tests passed separately in each
repository or in one combined run.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d5617ba9-11d8-4e0b-9435-e1b42c3a6ec9
📒 Files selected for processing (23)
.agent/memory/candidates/graduated/1a6dc6bccad0.json.agent/memory/candidates/graduated/446162929cb3.json.agent/memory/candidates/graduated/720dedfdd2c8.json.agent/memory/candidates/graduated/cd51f7e6bf6c.json.agent/memory/candidates/graduated/d1f3789d4592.json.agent/memory/episodic/AGENT_LEARNINGS.jsonl.agent/memory/semantic/DECISIONS.md.agent/memory/semantic/LESSONS.md.agent/memory/semantic/lessons.jsonl.agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md.agent/memory/working/PR_BODY_GRANT_HMAC_DECISIONS_2026-08-02.jsonl.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md.agent/memory/working/WORKSPACE.md.githooks/pre-commit.githooks/pre-pushscripts/cursor/append-pr-body.shscripts/cursor/grant-pr-body-human-override.shscripts/cursor/hooks/pr-body-guard-core.pyscripts/cursor/pr-body-grant-lib.pyscripts/git/guard-sync-manifest.shscripts/git/scrub_dsstore.shtests/test_append_pr_body_grant_flow.pytests/test_pr_body_grant_lib.py
🚧 Files skipped from review as they are similar to previous changes (6)
- scripts/cursor/grant-pr-body-human-override.sh
- tests/test_append_pr_body_grant_flow.py
- .githooks/pre-push
- scripts/cursor/append-pr-body.sh
- tests/test_pr_body_grant_lib.py
- scripts/cursor/hooks/pr-body-guard-core.py
Remediation, verified against current code before fixing each finding: - lessons.jsonl + graduated candidates: removed remaining /tmp literals from lesson_1a6dc6bccad0's rationale AND decisions[1].notes (2 fields, same file, review only caught one on first pass); reworded lesson_d1f3789d4592's claim from "Human-authorized" to "operator-gated" since the HMAC grant proves operator action, not human identity. - HMAC payload terminology: digest -> content_digest, verified against the actual implementation as canonical. - DECISIONS.md: excluded from MD013 (matching SKILL.md/LESSONS.md precedent, confirmed append-only/machine-grown), then fixed the remaining ~10 blank-line/bare-URL issues directly; also corrected content describing "gh pr edit" as a direct agent step when it's actually internal to append-pr-body.sh. - WORKSPACE.md's ambiguous "26 passed (orama + PT)" claim: actually ran the real test command in both repos separately (26 confirmed real in each, not a stale copy-paste) and rewrote the doc to state this unambiguously. - .githooks/pre-commit: fixed a real fail-open gap -- hygiene script present but python3 missing was silently falling through to a much narrower identity-only check with no warning. Now fails closed. - pr-body-grant-lib.py: pr_number had zero validation anywhere before reaching the canonical HMAC payload; repo's validator only checked for pipe characters, not newlines. Added _validate_pr_number, extended _validate_repo_slug, wired both into mint_grant and verify_grant_fields. 3 new regression tests, one initially exposed a real subtlety: a naive first draft using an inactive override made the test pass regardless of whether the fix was present, since the first command already denied on its own -- corrected by testing with the real bypass condition documented in this commit's own lesson below. Also fixed 5 pre-existing markdownlint violations pulled into scope on the SAGA doc once this commit touched it -- hand-authored narrative, fixed properly rather than excluded. 4 lessons graduated (checked existing lessons first to avoid re-capturing anything already recorded): - lesson_a791e2ee5ad6: the exclude-vs-fix decision for lint violations in machine-rendered vs hand-authored files, generalized from this session's concrete applications. - lesson_9cc7ae1a5f12: verifying findings against current code before fixing catches already-resolved/stale findings, saving real effort. - lesson_4c65d5956822: shell-command guards must split on bare newlines too, not just &&/;/||, and the specific way a badly-mocked test can pass without proving anything about the real vulnerability. - lesson_5fb495abcfbc: a combined existence-and-availability guard condition silently downgrades security coverage when the dependency is missing but the primary script exists -- separate the conditions. 29/29 tests pass (full suite: grant lib, append flow, guard core, divergence checker). 985 lessons.jsonl lines, zero duplicate IDs, all 4 new lessons' episodic mirrors confirmed. Hygiene clean.
… much older version than tested locally The earlier .markdownlint-cli2.jsonc fix used an overrides/filter/combine block that worked when tested with a locally-installed markdownlint-cli2 (v0.23.2, markdownlint v0.41.1) but CI's pinned davidanson/markdownlint-cli2-action@v19 actually runs markdownlint-cli2 v0.17.2 (markdownlint v0.37.4) -- a much older version that doesn't support that config schema at all. It silently ignored the override and CI failed on the full, unexempted DECISIONS.md (68 violations). Caught by installing the exact CI-matching version locally (npm install -g markdownlint-cli2@0.17.2) rather than trusting the newer local install, and confirmed empirically that the simpler key works correctly across both versions. Replaced overrides with ignores for DECISIONS.md/LESSONS.md/SKILL.md; reverified clean against the CI-matching version, not just the newer local one. Also fixed 2 genuine pre-existing MD013 violations in CODERABBIT_REVIEW_WAVE_...md, pulled into scope once this commit touched it -- hand-authored narrative, fixed directly. 29/29 tests pass. Hygiene clean. Reverified with markdownlint-cli2 v0.17.2 specifically (the real CI version), not the newer local default.
…int overrides - lesson_d1f3789d4592 supersedes lesson_4c914783ec46 in lessons.jsonl - Replace ignores with MD013 overrides for SKILL.md and skills tree (CodeRabbit #320 review 4837623699)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agent/memory/semantic/LESSONS.md:
- Line 29: Update the legacy lesson record identified by
lesson_legacy_df758c2d1245: either restore its superseded plaintext rule and add
superseded_by=lesson_d1f3789d4592 metadata, or remove the duplicate entry
entirely; do not leave the current operator-grant-v2 rule under status=legacy.
In @.markdownlint-cli2.jsonc:
- Around line 13-16: Update the ignores configuration in
.markdownlint-cli2.jsonc so DECISIONS.md, LESSONS.md, and SKILL.md are excluded
only from the intended rule rather than globally; use exact generated paths or a
scoped MD013 override, and ensure bin/orama-system/SKILL.md remains checked by
MD013.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c3fb4f6-6ee6-4abb-81ef-4df272741b67
📒 Files selected for processing (18)
.agent/memory/candidates/graduated/1a6dc6bccad0.json.agent/memory/candidates/graduated/4c65d5956822.json.agent/memory/candidates/graduated/5fb495abcfbc.json.agent/memory/candidates/graduated/9cc7ae1a5f12.json.agent/memory/candidates/graduated/a791e2ee5ad6.json.agent/memory/candidates/graduated/d1f3789d4592.json.agent/memory/episodic/AGENT_LEARNINGS.jsonl.agent/memory/semantic/DECISIONS.md.agent/memory/semantic/LESSONS.md.agent/memory/semantic/lessons.jsonl.agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.md.agent/memory/working/PR_BODY_GRANT_HMAC_DECISIONS_2026-08-02.jsonl.agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md.agent/memory/working/WORKSPACE.md.githooks/pre-commit.markdownlint-cli2.jsoncscripts/cursor/pr-body-grant-lib.pytests/test_pr_body_grant_lib.py
🚧 Files skipped from review as they are similar to previous changes (10)
- .agent/memory/candidates/graduated/d1f3789d4592.json
- .agent/memory/working/PR_BODY_GRANT_HMAC_DECISIONS_2026-08-02.jsonl
- .agent/memory/candidates/graduated/1a6dc6bccad0.json
- .agent/memory/working/WORKSPACE.md
- tests/test_pr_body_grant_lib.py
- .githooks/pre-commit
- .agent/memory/episodic/AGENT_LEARNINGS.jsonl
- .agent/memory/working/PR_BODY_GRANT_HMAC_MVP_SAGA_2026-08-02.md
- scripts/cursor/pr-body-grant-lib.py
- .agent/memory/semantic/DECISIONS.md
| - Operator-gated PR body edits require operator-grant-v2 HMAC ack from grant-pr-body-human-override.sh matching append bytes -- not CURSOR_PR_BODY_HUMAN_OVERRIDE_ACK env exports or operator-grant-v1 plaintext ack. The HMAC grant authorizes the append-pr-body.sh capability itself; it does not prove human identity or presence -- the underlying grant script's TTY-presence check is not a reliable human-vs-agent signal. <!-- status=accepted confidence=0.6 evidence=1 id=lesson_d1f3789d4592 --> | ||
| - Tracked .agent memory and working chronicles must not embed ephemeral scratch path literals — repo_hygiene flags them like workstation home paths and blocks PT CI. <!-- status=accepted confidence=0.6 evidence=1 id=lesson_1a6dc6bccad0 --> | ||
| - When a markdownlint (or similar whole-file linter) violation surfaces in a file pulled into scope by an unrelated edit, the correct response depends on the file's nature, not a blanket policy: machine-rendered, append-only historical records (a lessons/decisions log where entries are added mechanically over time, meant to be terse and grow indefinitely) should be excluded from the specific rule that doesn't meaningfully apply (e.g. line-length on a log that append-only tooling writes one dense entry per line), via a scoped config override -- not hand-reformatted, since doing so fights the tool that generates the file. Genuinely hand-authored prose (a narrative doc, a plan, a README) should have violations fixed properly, even if the count is nontrivial, since the rule is actually serving its purpose there. Determine which kind of file it is by reading its actual content and structure (does it read as authored narrative, or as accumulated terse entries with a consistent machine-written shape) before choosing exclude vs. fix, not by file location or extension alone. <!-- status=accepted confidence=0.6 evidence=1 id=lesson_a791e2ee5ad6 --> | ||
| - Human-authorized PR body edits require operator-grant-v2 HMAC ack from grant-pr-body-human-override.sh matching append bytes — not CURSOR_PR_BODY_HUMAN_OVERRIDE_ACK env exports or operator-grant-v1 plaintext ack. <!-- status=legacy confidence=0.7 evidence=0 id=lesson_legacy_df758c2d1245 --> |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Correct the legacy record metadata and content.
Line 29 is marked status=legacy, but its text describes the current operator-grant-v2 rule. It also lacks the superseded_by back-reference used by the append-only record rule. Restore the superseded plaintext rule with superseded_by=lesson_d1f3789d4592, or remove this duplicate entry.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agent/memory/semantic/LESSONS.md at line 29, Update the legacy lesson
record identified by lesson_legacy_df758c2d1245: either restore its superseded
plaintext rule and add superseded_by=lesson_d1f3789d4592 metadata, or remove the
duplicate entry entirely; do not leave the current operator-grant-v2 rule under
status=legacy.
| "ignores": [ | ||
| "**/DECISIONS.md", | ||
| "**/LESSONS.md", | ||
| "**/SKILL.md" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## tracked target file"
if [ -f .markdownlint-cli2.jsonc ]; then
cat -n .markdownlint-cli2.jsonc
else
echo "missing .markdownlint-cli2.jsonc"
fi
echo
echo "## repository markdownlint config and references"
git ls-files | rg '(^|/)(\.markdownlint-cli2\.jsonc|\.markdownlint(_rc)?\.(json|c?js|js|mjs|ts|yaml|yml)|package\.json|pnpm-lock\.yaml|package-lock\.json|yarn\.lock|$)' | sed -n '1,120p'
echo
echo "## relevant ignores/md013 references"
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' 'markdownlint-cli2|MD013|ignores|LESSONS|DECISIONS|SKILL' . -S | sed -n '1,220p'Repository: diazMelgarejo/Perpetua-Tools
Length of output: 33293
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## markdown workflow"
cat -n .github/workflows/markdown-lint.yml | sed -n '1,100p'
echo
echo "## markdownlint-cli2-action v19 README excerpts from repository or npm package docs"
python3 - <<'PY'
from urllib.request import urlopen
urls = [
"https://raw.githubusercontent.com/DavidAnson/markdownlint-cli2-action/v19/README.md",
" https://raw.githubusercontent.com/DavidAnson/markdownlint-cli2/main/jsonconfig/README.md",
]
for url in urls:
url = url.strip()
try:
text = urlopen(url, timeout=20).read().decode()
print(f"\n### {url}")
for i,line in enumerate(text.splitlines(),1):
if "ignores" in line.lower() or "config" in line.lower() or "globs" in line.lower():
print(f"{i}: {line}")
except Exception as e:
print(f"failed to fetch {url}: {e}")
PYRepository: diazMelgarejo/Perpetua-Tools
Length of output: 3734
🌐 Web query:
DavidAnson markdownlint-cli2 ignores option file globs disables linting documentation
💡 Result:
In markdownlint-cli2, the ignores option accepts an array of glob strings to specify files or directories that should be excluded from linting [1][2][3]. This setting does not disable linting documentation itself, but rather controls which files are scanned [1][2][3]. Performance characteristics of the ignores option depend on where it is defined: 1. Root Level: When defined in a configuration file located in the directory from which markdownlint-cli2 is run, the glob expressions are negated (prefixed with!) and appended to the command-line arguments before file enumeration [1][2][3]. This is the most performant method [1][3]. 2. Subdirectory Level: When applied in subdirectories, the ignores are processed after file enumeration, which can negatively impact performance when scanning large directory trees [1][3]. Nested configuration files inherit and reapply these settings to their respective subdirectories [1][3]. Additionally, markdownlint-cli2 supports a gitignore option, which, when enabled (e.g., gitignore: true), automatically imports ignore patterns from.gitignore files in the directory tree [3][4][5]. This is also most performant when used at the root level [3]. Users should note that the command-line argument --no-globs affects the top-level globs property (which defines files to include), not the ignores property [1][2][3].
Citations:
- 1: https://www.npmjs.com/package/markdownlint-cli2
- 2: https://github.com/DavidAnson/markdownlint-cli2
- 3: https://cdn.jsdelivr.net/npm/@toolbox-ts/types@0.3.0/dist/configs/definitions/markdownlint/cli2.d.ts
- 4: https://github.com/DavidAnson/markdownlint-cli2/blob/HEAD/test/markdownlint-cli2-jsonc-example/.markdownlint-cli2.jsonc
- 5: https://github.com/DavidAnson/markdownlint-cli2/blob/main/test/markdownlint-cli2-yaml-example/.markdownlint-cli2.yaml
Scope ignores to the intended files.
**/DECISIONS.md, **/LESSONS.md, and **/SKILL.md exclude every matching file from linting, not only MD013. The **/SKILL.md pattern also skips the documented bin/orama-system/SKILL.md location where MD013 should still be checked. Use exact generated paths or a scoped rule override.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.markdownlint-cli2.jsonc around lines 13 - 16, Update the ignores
configuration in .markdownlint-cli2.jsonc so DECISIONS.md, LESSONS.md, and
SKILL.md are excluded only from the intended rule rather than globally; use
exact generated paths or a scoped MD013 override, and ensure
bin/orama-system/SKILL.md remains checked by MD013.
…itleaks debugging session 3 lessons graduated: - lesson_77ce859d6970: verify CI-tool config fixes against the exact pinned tool version CI actually runs, not the newest locally-installed version -- a config schema can silently work locally and do nothing in CI if the actually-pinned version doesn't support it. - lesson_084412e4e566: secret scanners that scan full commit history (not just current tree state) can keep flagging already-superseded commits whose problematic content was fixed later on the same branch -- the fix is a scoped commit-SHA allowlist, not another file edit. - lesson_5f0362a3f95f: never extend a short git SHA by guessing/padding hex characters -- always git rev-parse it. A fabricated SHA in a security-tool allowlist silently matches nothing while looking syntactically correct. Plus a compact narrative report (2026-08-02-ci-tool-version-and-gitleaks-remediation-report.md) of the actual debugging arc across both repos, ending in a reusable checklist for the next CI-tool-config fix. Checked existing lessons first (2 incidental keyword matches, both unrelated on inspection) before graduating anything, to avoid duplicating already-captured claims. 975 lessons.jsonl lines, zero duplicate IDs, all 3 new lessons' episodic mirrors confirmed. Report itself verified with the CI-matching markdownlint-cli2 v0.17.2, not the newer local default -- applying the lesson it documents. Hygiene clean.
|
Review fixes pushed (
|
Union lessons.jsonl + episodic; re-render LESSONS.md. Dangling commit was on 2026-08-02-ci-version-mismatch-lessons only.
|
Paired-branch sync (2026-08-02): Dangling-commit pattern: |
A later commit on this branch (1ccca81, applying orama PR #320 review 4837623699) replaced the working ignores-based MD013 exemption with the overrides/filter/combine schema already proven this session not to work against CI's actually-pinned markdownlint-cli2 v0.17.2 -- reintroducing the exact CI failure that had already been diagnosed and fixed. Restored the ignores-based config, verified against the exact CI-pinned tool version (markdownlint-cli2 v0.17.2, not the newer local default): SKILL.md, docs/LESSONS.md, and .agent/memory/semantic/DECISIONS.md all correctly exempted, 0 issues. 29/29 tests pass. Hygiene clean.
Compared PT's PR #320 branch against orama's PR #260 branch (the source of truth for this session's pr-body-guard/grant work) across the full guard-script ecosystem -- scripts/cursor/hooks/*, scripts/cursor/ {append-pr-body,grant-pr-body-human-override,pr-body-grant-lib}.*, scripts/cursor/hooks.json.template, scripts/git/check-guard-sync- divergence.sh, and the matching test files. Several were already synced from earlier work this session on this same branch; this pass filled the remaining gap -- PT was missing the entire grant-v2 HMAC library (pr-body-grant-lib.py) and its human-override grant script entirely, and had a stale copy of pr-body-guard-core.py without the newline- segment-splitting security fix from orama PR #251/review 4835288649. Synced exactly (byte-identical from orama's source), permissions matched to the established pattern (0755 for executable scripts, 0644 for the hooks.json.template data file). .gitleaks.toml intentionally NOT propagated -- checked first: PT's CI has no gitleaks job at all, so the config would be inert. 26/26 tests pass on the synced files. Confirmed a broad, pre-existing test-suite failure (259 failures across unrelated files like test_gossip_bus.py) predates this sync entirely -- verified via git stash before/after comparison, not assumed -- and is out of scope for this propagation. Hygiene clean.
…PR 260's review remediation Postmortem (2026-08-02-dispatch-race-deferral-and-config-scope- postmortem.md), 4 parts: the deferral-date internal-consistency bug in the dispatch-race work; the duplicate-fallback fix and its own subtle near-miss (a naive broader fix would have regressed a different, valid scenario); the markdown config-scope investigation that verified a precise fix works but surfaces 1032 pre-existing violations across 205 files, not adopted for that reason; and the completed root-cause diagnosis of a large pre-existing test-failure count an earlier note had only confirmed as unrelated without explaining. 3 lessons graduated, checked against existing lessons first (0 matches on the specific patterns): - lesson_61d719978435: verify two halves of a gated-feature commit are actually consistent with each other via the test that exercises both, not just each half's own coverage. - lesson_64d1deca2fca: a duplicate-removal fix can be correct in one code state and a regression in another -- check before removing unconditionally. - lesson_8bf6b00d2a44: finish the diagnosis of a confirmed-unrelated failure count where time allows; 'pre-existing, unrelated' without the actual cause leaves the next reader to re-investigate from scratch. Scanned for personal-path or workstation-identity literals before writing -- none present; all paths described repo-relatively. 994 lessons.jsonl lines, zero duplicate IDs, all 3 mirrors confirmed. Hygiene clean.


Post-#319 follow-up syncing orama PR #255 CodeRabbit reviews
4835024659and4835288649.Guard-sync (commits
dd8bcba3,458a5ab9)git -C … rev-parse$# -eq 1); fail-closed when checker missingscripts/git/touchesPR-body Layer 0 (commits
2afd4044,88783688)grant-pr-body-human-override.sh— operator TTY grant, 8h TTL ack fileupdate_pr,gh pr edit,gh api); onlyappend-pr-body.shafter grant.cursor/rules/pr-body-comment-only.mdcandremind-pr-body-append-only.shMemory + lessons
.agent/memory/working/CODERABBIT_REVIEW_WAVE_4835024659_4835288649_2026-08-01.mdlesson_64e972d330c2,lesson_82c94865243e,lesson_8c5f6349aa87orama #255 branch already at
88783688(pushed).Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests